Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure execution_context is propagated to additional_codecs #152

Merged
merged 4 commits into from
Apr 12, 2022

Conversation

yaauie
Copy link
Contributor

@yaauie yaauie commented Apr 11, 2022

This solves the issue where a pipeline is started with pipeline.ecs_compatibility set to a value other than the global value for that setting, and the additional_codecs instantiated by this plugin do not respect the pipeline setting.

It does so by reaching into Logstash internals IFF the necessary ::LogStash::Plugins::Contextualizer is present.

In the long-run, I would prefer to provide a plugin factory from Logstash core (see: logstash-plugins/logstash-mixin-plugin_factory_support#1), but this solves the immediate need for this particular plugin.

Copy link
Member

@jsvd jsvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good catch! LGTM, with a very tiny cosmetic suggestion.

lib/logstash/inputs/http.rb Outdated Show resolved Hide resolved
too complex to be a guard clause.

Co-authored-by: João Duarte <jsvd@users.noreply.github.com>
@yaauie yaauie merged commit 0e37a1c into logstash-plugins:main Apr 12, 2022
yaauie added a commit to yaauie/logstash-input-http that referenced this pull request Apr 12, 2022
…ogstash-plugins#152)

* ensure execution_context is propagated to additional_codecs

* more closely specify the desired behaviour for pipeline.ecs_compatibility

* version bump

* restructure conditional

too complex to be a guard clause.

Co-authored-by: João Duarte <jsvd@users.noreply.github.com>

Co-authored-by: João Duarte <jsvd@users.noreply.github.com>
@yaauie yaauie deleted the codec-context branch April 12, 2022 15:37
@yaauie yaauie mentioned this pull request Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants